-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix appwebpath detection when OC installed into the subdir and approot is a sibling of oc root dir #31175
Conversation
lib/private/App/AppManager.php
Outdated
@@ -531,7 +531,11 @@ public function getAppWebPath($appId) { | |||
$appRoot['url'] = substr($appRoot['url'], 3); | |||
$ocWebRoot = dirname($ocWebRoot); | |||
} | |||
return $ocWebRoot . '/' . ltrim($appRoot['url'], '/'); | |||
return sprintf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so no way to just use trim()
instead of this exotic approach ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PVince81 we need to concatenate to two parts with slash and both of them potentially could already have trailing and leading slash respectively.
this is not exotic as we officially prefer interpolation ;)
indeed it can be rewritten like this
$trimmedOcWebRoot = rtrim($ocWebRoot, '/');
$trimmedAppRoot = ltrim($appRoot['url'], '/');
return "$trimmedOcWebRoot/$trimmedAppRoot";
Codecov Report
@@ Coverage Diff @@
## master #31175 +/- ##
============================================
+ Coverage 62.55% 62.55% +<.01%
Complexity 18279 18279
============================================
Files 1147 1147
Lines 68472 68474 +2
Branches 1234 1234
============================================
+ Hits 42831 42833 +2
Misses 25280 25280
Partials 361 361
Continue to review full report at Codecov.
|
@PVince81 ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 code looks good, please backport for RC3
Stable10: #31178 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Fixes css/js resolution for a case when owncloud installed into the first descendant of the server root directory and app-theme is located outside owncloud root directory.
Related Issue
Fixes #31170
Motivation and Context
rewritten code produced double slash in the case described above and this broke proper URL generation for themes
How Has This Been Tested?
Steps are listed in the issue #31170 (comment)
Types of changes
Checklist: